-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Increase reliability of host network setup #310
Conversation
Handle if /etc/resolv.conf doesn't exist on system to migrate or if it is a symlink
if os.path.islink(resolv_conf): | ||
log.info('{0} is a symlink, renaming it to {0}.pre-migration, bind mounting /etc/resolv.conf to {0}'.format(resolv_conf)) | ||
os.replace(resolv_conf,resolv_conf+'.pre-migration') | ||
open(resolv_conf,'w').close() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But if resolv.conf
is a symlink then the user could have pointed it to any location that has resolution information. If we stick an empty file in it's place there is a good chance that network access will not work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By default on SLES15, /etc/resolv.conf is a symlink to /run/netconfig/resolv.conf (in tmpfs), which is not available on SLES16. This is why we need to move the symlink away (we can't bind mount on top of a symlink).
On SLES16, /etc/resolv.conf is no longer a symlink but a file, which is managed by NetworkManager. If the file is empty, it will replaced by NetworkManager by the information from DHCP (or NM configuration).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we at least check if the symlink points to the default location? If it doesn't we should probably not write an empty file and instead copy the content from the pointed to file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, I'll add some checks to see if the symlink is resolvable (ie not pointing to /run). If it is, it should be left intact, if not, we could do the renaming.
WDYT ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think if the symlink points to the default location we can be reasonably certain that the user has not added any custom configuration and that a dhcp based setup will work. If it it points somewhere else we should read the content of the target and dump that in the new file being created.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why should bind mounting a symlink be a problem ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the various issues the patch is trying to resolve are due to /etc/resolv.conf symlink to /run/netconfig/resolv.conf being broken on a SLES 16 system (ie the migration live image), since netconfig is not present anymore, nor wicked.
- test on /etc/resolv.conf existence fails (os.path.exists will follow the link)
- bind mount on /etc/resolv.conf symlink will fail too (symlink target doesn't exist)
I'm not super happy with touching the migration system either, before we start the migration process.
I'll test another approach, by creating a symlink /run/netconfig/resolv.conf in the sle16 migration image and reports if it works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fcrozat as we control the migration environment I would also be in favor to "fix" the migration live image and the container to provide a proper environment rather than adding code to do this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fcrozat something different. I see you work from forks and for whatever reason I cannot make the github actions to run from your contribution. Normally I have a button to "approve and run" the actions but I believe it's because the SUSE org is a private one this doesn't work. It would be better to add you to the project and you can work from branches which will then also run the actions and tests. I believe I don't have permissions to add members but maybe @rjschwei or @aosthof can do this ? Thanks much
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't want to bother you with my initial tests but I'm fine following your regular workflow.
Replaced by #311 (and another upcoming PR) |
Handle if /etc/resolv.conf doesn't exist on system to migrate or if it is a symlink
This is required to migrate SLE 15 system and should be safe on SLE12 ones (untested)